Conversation
kevinjqliu
left a comment
kevinjqliu
left a comment
There was a problem hiding this comment.
Thanks for adding this feature!
I think it would be great to include a E2E test with spark. I believe the feature will be available in the next java iceberg release, 1.10
|
@kevinjqliu happy to add that e2e test! Would you prefer I wait for the release + add the test or just file an issue? I know v3 writing is blocked on this feature. |
rambleraptor
force-pushed
the
encryption-key
branch
from
86543cb to
eceff33
Compare
smaheshwar-pltr
left a comment
smaheshwar-pltr
left a comment
There was a problem hiding this comment.
Thanks for picking up this work!
| assert ( | ||
| repr(snapshot) | ||
| == """Snapshot(snapshot_id=25, parent_snapshot_id=19, sequence_number=200, timestamp_ms=1602638573590, manifest_list='s3:/a/b/c.avro', summary=Summary(Operation.APPEND), schema_id=3)""" | ||
| == """Snapshot(snapshot_id=25, parent_snapshot_id=19, sequence_number=200, timestamp_ms=1602638573590, manifest_list='s3:/a/b/c.avro', summary=Summary(Operation.APPEND), schema_id=3, key_id=None)""" |
There was a problem hiding this comment.
Perhaps we want to omit key_id for < V3 tables instead of serialising it.
The spec says:
with blanks for format versions 1 and 2. #2146 (comment) then makes me think we shouldn't write this field for those versions. WDYT?
There was a problem hiding this comment.
#1973 makes it sound like we don't want to version these. What are your thoughts?
|
Now that apache/iceberg#7770 got merged, it would be fantastic if we had support for reading encrypted tables in pyiceberg. Is this PR still active? If not i can take a stab at it. |
|
I can take a look at it! I've been waiting for 1.10. Thanks for the ping |
|
@kevinjqliu what would the e2e test for this look like? I'm not sure how to run these events using Spark. |
rambleraptor
force-pushed
the
encryption-key
branch
from
eceff33 to
7d0985b
Compare
4 participants
Closes #1972
Rationale for this change
This adds support for adding / removing encryption keys on table metadata + snapshots. This is a new part of v3. The goal is to match the Java implementation.
Spec change
Java change
Are these changes tested?
Included unit tests
Are there any user-facing changes?
Yes, this allows new support for encryption key metadata.